fix: handling of target for guides, tests using lgtm.toml#147
Conversation
7841289 to
0238664
Compare
0238664 to
8384a1e
Compare
|
/lgtm review |
There was a problem hiding this comment.
🦉 lgtm Review
Score: Nitpicks 🤓
🔍 Summary
This is a solid PR that improves the codebase in several ways. The refactoring of the TARGET argument into a configurable TargetParser class is a great improvement for modularity, and the fix to prevent local repositories from being used with the guide command is well-implemented. The changes to improve test isolation are also a welcome addition for the long-term health of the test suite.
However, there is one blocking issue in the evaluate_review_quality.py script. A call to the refactored _parse_target function is missing a required argument, which will cause the script to fail.
Please address the comment below. Once fixed, this PR will be in great shape to merge.
More information
- Id:
c665bd38d23e424c8a5db2c47c33bc66 - Model:
gemini-2.5-pro - Created at:
2025-09-29T07:48:10.376705+00:00
Usage summary
- Request count:
2 - Request tokens:
31,072 - Response tokens:
6,416 - Total tokens:
37,488
See the 📚 lgtm-ai repository for more information about lgtm.
This PR was originally just attempting to add issues integration to our lgtm action in the repo.
However, it uncovered that some tests were using the
lgtm.tomlfile of the repo itself.Fixing that (with a solution that was already used in part of the test suite), uncovered an issue in handling
TARGETfor reviewer guides: we were accepting git repositories as target, and so a good error message was not displayed to the user.This PR fixes all these issues.